Skip to content

Conversation

@mina1957
Copy link

@mina1957 mina1957 commented Nov 7, 2025

@mina1957 mina1957 requested a review from rhshadrach as a code owner November 7, 2025 22:08
Copy link
Member

@rhshadrach rhshadrach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. Overall, this seems far too complex an implementation for the feature being implemented. Why can we not just determine the format upon reading each cell and react appropriately?

In addition, it does not seem to me pandas should ever read a cell as numeric if that cell is text. I think we should not implement this as a flag.

ordered_levels.append(level_idx)
return ordered_levels

def _convert_index_labels(self, index, levels_to_convert: list[int]):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like code got duplicated?

ordered_levels.append(level_idx)
return ordered_levels

def _convert_index_labels(self, index, levels_to_convert: list[int]):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks the same?

Comment on lines +3342 to +3344
This behavior currently applies to the ``openpyxl`` and ``xlrd`` engines. Other
engines simply ignore the flag until text format detection is implemented for
them.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm negative on diverging behavior between readers unless absolutely necessary.

Comment on lines +1003 to +1014
@staticmethod
def _parser_engine(parser):
return getattr(parser, "_engine", parser)

@classmethod
def _parser_attr(cls, parser, attribute: str):
if hasattr(parser, attribute):
return getattr(parser, attribute)
engine = cls._parser_engine(parser)
if engine is not parser and hasattr(engine, attribute):
return getattr(engine, attribute)
return None
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these necessary?


@staticmethod
def _cell_is_text_formatted(cell) -> bool:
number_format = getattr(cell, "number_format", None)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When does cell have and not have the number_format attribute?

@rhshadrach rhshadrach added the IO Excel read_excel, to_excel label Nov 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

IO Excel read_excel, to_excel

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ENH: Columns formatted as "Text" in Excel are read as numbers

2 participants